Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allows configurable iteration order in AggregateDataSource, and adds a configurable version #125

Merged
merged 2 commits into from
Apr 14, 2021

Conversation

Craigacp
Copy link
Member

@Craigacp Craigacp commented Apr 2, 2021

Description

Adds an enum to AggregateDataSource that lets users pick the iteration order. The two options are sequential, which was the old behaviour, where each internal data source is exhausted in sequence, or round robin which draws an example from each data source in turn.

Adds AggregateConfigurableDataSource which is the same as AggregateDataSource but can be configured.

There's also a slight refactor in the test helpers to expose provenance marshalling checks.

Motivation

Building data sources from multiple files is a pain, and AggregateDataSource had slightly misleading documentation. This fixes the docs and makes it easier to aggregate things as they can be all placed in one config file.

Partial fix for #123 until we have a bigger refactor of file based data sources.

@Craigacp Craigacp added the Oracle employee This PR is from an Oracle employee label Apr 13, 2021
Copy link
Member

@eelstretching eelstretching left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks good. Might want to add a test with an empty data set, though.

@Craigacp
Copy link
Member Author

Ok, I'll add that in the pass when we upgrade OLCUT as I'll be back in here removing the todos.

@Craigacp Craigacp merged commit 1f832c9 into main Apr 14, 2021
@Craigacp Craigacp deleted the aggregate-data-source branch April 14, 2021 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Oracle employee This PR is from an Oracle employee
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants